Skip to content

pre-commit upgrade codespell and fix spelling #10144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jbampton
Copy link
Member

Description

This PR...

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Copy link

codecov bot commented Dec 28, 2024

Codecov Report

Attention: Patch coverage is 12.82051% with 34 lines in your changes missing coverage. Please review.

Project coverage is 16.06%. Comparing base (b48de4e) to head (b833c07).
Report is 26 commits behind head on main.

Files with missing lines Patch % Lines
...cloud/storage/resource/VmwareStorageProcessor.java 0.00% 10 Missing ⚠️
...om/cloud/deploy/DeploymentPlanningManagerImpl.java 0.00% 6 Missing ⚠️
...e/resource/StorageSubsystemCommandHandlerBase.java 0.00% 1 Missing and 1 partial ⚠️
...ud/hypervisor/kvm/storage/KVMStorageProcessor.java 0.00% 2 Missing ⚠️
.../xenserver/resource/XenServerStorageProcessor.java 0.00% 2 Missing ⚠️
...nserver/resource/Xenserver625StorageProcessor.java 50.00% 2 Missing ⚠️
...n/java/com/cloud/storage/VolumeApiServiceImpl.java 0.00% 2 Missing ⚠️
.../CheckDataStoreStoragePolicyComplianceCommand.java 0.00% 1 Missing ⚠️
...torage/allocator/AbstractStoragePoolAllocator.java 0.00% 1 Missing ⚠️
...ypervisor/ovm3/resources/Ovm3StorageProcessor.java 0.00% 1 Missing ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff            @@
##               main   #10144   +/-   ##
=========================================
  Coverage     16.06%   16.06%           
- Complexity    12871    12872    +1     
=========================================
  Files          5642     5642           
  Lines        493973   493973           
  Branches      59895    59895           
=========================================
+ Hits          79348    79351    +3     
+ Misses       405839   405837    -2     
+ Partials       8786     8785    -1     
Flag Coverage Δ
uitests 4.01% <ø> (ø)
unittests 16.90% <12.82%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jbampton jbampton closed this Dec 28, 2024
@jbampton jbampton reopened this Dec 28, 2024
Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could refuse a lot of those spellos but lgtm like this as well (non native ;)

@jbampton
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@jbampton a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11930

@DaanHoogland
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11939

@@ -35,6 +35,7 @@ aqcuire
aqcuired
aquire
aquiring
assertin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any usages of assertin, callin, dockin and notin

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any usages of assertin, callin, dockin and notin

The codespell.txt ignored words list is all lowercase but in our codebase it finds both for example:

self.assertIn(f"Failed to allocate

Copy link
Member

@vishesh92 vishesh92 Jan 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbampton got it. But can you remove fals & ources after fixing those. I can only find 1 instance of these words.

@@ -211,6 +216,7 @@ faield
faild
failes
falied
fals
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can only see one usage of fals which is in plugins/hypervisors/hyperv/DotNet/ServerResource/WmiWrappers/ROOT.virtualization.v2.Msvm_ExternalEthernetPort.cs. Better fix in the file and remove fals from here.

@@ -339,6 +347,7 @@ opeation
optin
orginal
otherwse
ources
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can only see one usage of ources which is in plugins/hypervisors/hyperv/DotNet/ServerResource/WmiWrappers/ROOT.virtualization.v2.Msvm_EthernetSwitchPort.cs. Better fix in the file and remove ources from here.

Copy link

github-actions bot commented Jan 8, 2025

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants